fix: defer remote connect lease and metro setup#417
Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d57495ce63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (shouldAllocateLeaseForCommand(command)) { | ||
| const leaseBackend = state.leaseBackend ?? requireRequestedLeaseBackend(flags, command); | ||
| assertRequestedConnectionScope(state, flags, leaseBackend); | ||
| if (!state.leaseId || !state.leaseBackend) { | ||
| const lease = await allocateOrReuseLease(client, state, leaseBackend); |
There was a problem hiding this comment.
The current guard only calls allocateOrReuseLease when !state.leaseId || !state.leaseBackend, so existing persisted leases are never heartbeated/revalidated before use. This makes the reuse branch in allocateOrReuseLease effectively unreachable and causes stale lease IDs (for example after reconnecting to an older saved state) to be passed straight into normal commands, which can fail with unauthorized/expired lease errors instead of being renewed.
Useful? React with 👍 / 👎.
| function shouldAllocateLeaseForCommand(command: string): boolean { | ||
| return !leaseDeferredCommands.has(command); |
There was a problem hiding this comment.
shouldAllocateLeaseForCommand currently forces lease materialization for any command not in the deferred set, which includes close. After a minimal connect state with no resolved backend/lease, running close can now fail early with the backend requirement error before sessions.close executes, even though session close is intended to work without allocating a new lease (as shown by the disconnect flow).
Useful? React with 👍 / 👎.
thymikee
force-pushed
the
fix/deferred-remote-connect-profile
branch
4 times, most recently
from
e96abf2 to
2fd1f3f
Compare
thymikee
force-pushed
the
fix/deferred-remote-connect-profile
branch
from
2fd1f3f to
6173197
Compare
1 participant
Summary
connectprofiles by auto-generating local session names and deferring lease allocation plus Metro preparation until a later command needs themsession,platform,leaseBackend, and Metro overrides working while persisting remote connection state that can start without a resolved leasesrc/cli/commands/connection-runtime.tsso the connection command entrypoint stays focusedValidation
pnpm formatpnpm check:unit